feat: Adds dataAttributes support to ButtonDropdown items#4247
feat: Adds dataAttributes support to ButtonDropdown items#4247giridharsaday wants to merge 1 commit intocloudscape-design:mainfrom
Conversation
- Add dataAttributes property to Item interface (CheckboxItem inherits it) - Automatically prefix attribute names with 'data-' - Prevent 'testid' override to preserve existing behavior - Add comprehensive unit tests including checkbox items Resolves: AWSUI-61657
| * } | ||
| * } | ||
| * ]} | ||
| * // Renders as: data-analytics-action="edit-product" data-item-key="product-123" |
There was a problem hiding this comment.
These docs are not parsed by our docs extractor automatically. Please add it to the items prop description instead.
Let's make them less comprehensive also - I think a single sentence or two should be enough (it is important to mention the prefixing with data-.)
| * - Filters out undefined values | ||
| */ | ||
| const getDataAttributes = (dataAttributes?: Record<string, string>): Record<string, string> => { | ||
| if (!dataAttributes) return {}; |
There was a problem hiding this comment.
This fails with our linting rules. Please use the prettier/eslint configuration of the package. You can also run npm run lint locally to see if there are some remaining errors.
| return Object.entries(dataAttributes).reduce((acc, [key, value]) => { | ||
| // Exclude 'testid' to prevent overriding existing data-testid behavior | ||
| if (key === 'testid' || key === 'data-testid') { | ||
| console.warn('ButtonDropdown: "testid" key is reserved and cannot be overridden via dataAttributes'); |
There was a problem hiding this comment.
Please use warnOnce utility for that.
| * - Excludes 'testid' to prevent overriding existing behavior | ||
| * - Filters out undefined values | ||
| */ | ||
| const getDataAttributes = (dataAttributes?: Record<string, string>): Record<string, string> => { |
There was a problem hiding this comment.
nit: I would move this util to /src/internal/utils - it will make it then easy to reuse for future similar use cases.
The util can probably take a list of properties to exclude.
| /> | ||
| ); | ||
|
|
||
| const item = getByText('Edit').closest('li'); |
There was a problem hiding this comment.
Please use test-utils to find button dropdown items.
| expect(item).toHaveAttribute('data-item-key', 'product-123'); | ||
| }); | ||
|
|
||
| test('automatically prepends data- prefix', () => { |
There was a problem hiding this comment.
how is that different from the prev test?
There was a problem hiding this comment.
At the same time, there seems to be no test to ensure that the "data-" prefix is not added (again) when the passed property already has it.
| expect(item).not.toHaveAttribute('data-undefined'); | ||
| }); | ||
|
|
||
| test('works with multiple items', () => { |
There was a problem hiding this comment.
I think the "works with multiple items", "works with disabled items", etc - can be combined into a single tests with multiple different items.
Description
This PR adds support for custom data attributes on ButtonDropdown items, enabling teams to add analytics tracking and custom metadata without DOM manipulation workarounds.
Key Changes:
dataAttributesproperty to Item interface (CheckboxItem inherits it)Use Case:
Teams building analytics-instrumented UIs (like NGS Hub with Tricorder) need to add data-* attributes to track user interactions consistently across multiple instances of the same action.
API Design:
Related links:
How has this been tested?
<li>elementsReviewers can test by:
npm test -- src/button-dropdown/__tests__/data-attributes.test.tsxnpm startReview checklist
Correctness
Security
Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.